Skip to content

test(ui): poll for the copy failure state instead of a fixed tick budget - #3476

Merged
kwakayama merged 1 commit into
mainfrom
fix/code-block-copy-test-race
Aug 8, 2026
Merged

test(ui): poll for the copy failure state instead of a fixed tick budget#3476
kwakayama merged 1 commit into
mainfrom
fix/code-block-copy-test-race

Conversation

@kojiwakayama

@kojiwakayama kojiwakayama commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

code-block.test.tsx › exposes failed copy feedback to assistive technology failed once in CI on #3474 — a branch that changes one line of CSP caching and no React code — and passed on re-run.

The race. settle spends a fixed budget of one microtask and one macrotask:

await Promise.resolve();
await new Promise((resolve) => setTimeout(resolve, 0));
flushSync(() => {});

That covers the success path, where the clipboard promise resolves and React re-renders. The failure path is longer: a rejected clipboard.writeText falls back to execCommand, and only once that returns false is the failed state set. On a loaded machine the assertion can run against the pre-failure render. It explains the shape of the flake exactly — only the two failure-path tests are exposed, and the success-path ones never flake.

The change. Both failure-path tests poll for the state they are actually about instead of guessing a tick count. The helper throws a named timeout, so a genuine regression still fails loudly rather than being waited out — I checked that by pointing the predicate at a sentinel that is never true and confirming it reports Timed out waiting for the copy control to report failure rather than passing.

What I cannot claim. I could not reproduce the flake locally: not in isolation (5/5 green), not with the whole ui/ directory in one process, not under --coverage, and not on a pristine origin/main worktree. So this removes a race that is visible in the code, not one I was able to observe. If it recurs, the timeout message will now say which condition was never reached, which is more than the previous failure gave us.

Separate from #3474 on purpose — an unrelated test-harness change does not belong in a one-line behavioural fix.

Summary by CodeRabbit

  • Tests
    • Improved clipboard failure test reliability by waiting for asynchronous state updates.
    • Added timeout handling with clearer error feedback when expected updates do not occur.

`code-block.test.tsx` failed once in CI on the assistive-technology assertion
and passed on re-run, on a branch that touches no React code.

`settle` spends a fixed budget of one microtask and one macrotask. That covers
the success path, where the clipboard promise resolves and React re-renders.
The failure path is longer: a rejected `clipboard.writeText` falls back to
`execCommand`, and only once that returns false is the failed state set. On a
loaded machine the assertion can run against the pre-failure render, which is
why only the two failure-path tests are affected while the success-path ones
never flake.

Both now poll for the state under test rather than guessing how many ticks it
takes. The helper throws a named timeout, so a real regression still fails
rather than being waited out.

I could not reproduce the flake locally -- not in isolation, not with the whole
ui directory in one process, not under coverage -- so this removes a race that
is visible in the code rather than one I was able to observe.
@kojiwakayama
kojiwakayama requested a review from kwakayama as a code owner August 8, 2026 17:50
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The code-block tests add a polling helper for asynchronous state changes. Clipboard failure assertions now wait for the failed state and accessibility feedback instead of using a fixed delay.

Changes

Clipboard failure test synchronization

Layer / File(s) Summary
Polling-based failure assertions
src/react/components/ui/code-block.test.tsx
The tests add waitFor, which flushes updates and polls for expected state changes. Clipboard failure tests use it to wait for failed-copy state and accessibility feedback.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: kwakayama

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main test change: polling for copy failure state instead of using a fixed tick budget.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/code-block-copy-test-race

Comment @coderabbitai help to get the list of available commands.

@kwakayama
kwakayama enabled auto-merge August 8, 2026 17:52

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/react/components/ui/code-block.test.tsx`:
- Around line 76-82: Update waitFor to use a named timeout duration and a
deadline-based polling loop instead of the fixed 100-attempt limit. Continue
checking until the deadline is reached, and avoid sleeping after the final
failed predicate check before throwing the existing timeout error.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1c97af67-e0bb-432a-82fc-77fc532b07e2

📥 Commits

Reviewing files that changed from the base of the PR and between 83ee058 and cf8f3c5.

📒 Files selected for processing (1)
  • src/react/components/ui/code-block.test.tsx

Comment thread src/react/components/ui/code-block.test.tsx
@kwakayama
kwakayama added this pull request to the merge queue Aug 8, 2026
Merged via the queue into main with commit 1cdeef4 Aug 8, 2026
31 checks passed
@kwakayama
kwakayama deleted the fix/code-block-copy-test-race branch August 8, 2026 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants